Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for popstate firing on page load. #1425

Merged
merged 1 commit into from Oct 15, 2012
Merged

Conversation

ghempton
Copy link
Member

@ghempton ghempton commented Oct 3, 2012

On Chrome popstate is fired on page load (see http://stackoverflow.com/questions/6421769/popstate-on-pages-load-in-chrome). This causes the initial route to be entered twice. This PR fixes that.

I would add a test for this, but due it it requiring a fresh page load, it is very non-trivial.

@rlivsey
Copy link
Contributor

rlivsey commented Oct 3, 2012

I can confirm that this fixes the problem in my app, cheers

@wagenet
Copy link
Member

wagenet commented Oct 3, 2012

@ghempton Sounds good. I assume you've also tested this on browsers that don't trigger the initial popState?

@ghempton
Copy link
Member Author

ghempton commented Oct 3, 2012

@wagenet yup, I've tested on FF which doesn't trigger on page load.

@digitaltoad
Copy link
Contributor

I have a branch that I'm working on to use replaceState on HistoryLocation init. Would this still be an issue at that point?

@ghempton
Copy link
Member Author

ghempton commented Oct 3, 2012

@digitaltoad I think this fix will still be needed.

@wycats
Copy link
Member

wycats commented Oct 15, 2012

@digitaltoad what is the status of your branch?

wycats added a commit that referenced this pull request Oct 15, 2012
Fix for popstate firing on page load.
@wycats wycats merged commit 0a4dd24 into emberjs:master Oct 15, 2012
@digitaltoad
Copy link
Contributor

I haven't submitted it due to testing. Not really sure the best way to test that replaceState is used without using replaceState.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants